Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#69] Improves error message when the tag is not found #127

Closed
wants to merge 1 commit into from

Conversation

crudiedo
Copy link
Contributor

@crudiedo crudiedo commented Oct 4, 2022

Resolves #69

This PR improves error message when the tag is not found by adding new TagError error that displays a useful error message. Besides that, the error message may also display a suggestion (Did you mean?) by fetching the available tags and finding the similar one calculating Edit distance.

Additional tasks

  • Documentation for changes provided/changed
  • Tests added
  • Updated CHANGELOG.md

…refetch non-existing tag, find possible correct tag using edi distance
@crudiedo crudiedo requested a review from chshersh as a code owner October 4, 2022 08:27
@chshersh chshersh added the hacktoberfest-accepted https://hacktoberfest.com/participation/ label Oct 6, 2022
@chshersh
Copy link
Owner

chshersh commented Oct 6, 2022

This PR will take a while for me to review due to the implementation complexity and the need to take multiple difficult decisions so I'm lowering its priority in favour of reviewing other PRs.

I've added the label though. Will get back to this PR later when I have enough time.

@chshersh
Copy link
Owner

chshersh commented Oct 9, 2022

My proposal is to wait for the implementation of #128 first to see how it affects the design of this issue. As mentioned before, we would like to avoid the total number of GitHub API queries. So a potential solution would be to query GitHub API again for the list of all releases if we don't find the specified tag.

As for the specific implementation of this issue, I propose the following weights for the Levenstein distance algorithm:

  • EDIT_DISTANCE_THRESHOLD = 4 (as current)
  • Add weight: 1
  • Delete weight: 1
  • Replace weight: 2 (aka add + delete; or we can even remove the replace case entirely from the algorithm if it's present and it'll be simulated by add + delete automatically)

The idea behind this is to suggest similar tags. I think it would be weird to suggest version 7.8.9 when the user has specified 1.2.3. The common case is the absence or presence of the v prefix. Or a different minor version (e.g. 13.0.1 vs 13.1.0).

If there's no release within the edit distance threshold, we could just output all the possible releases with some of their info (e.g. the release date) so the users could select the desired release quickly.

@chshersh chshersh added the output Fancy (and not so) output of the tool label Oct 9, 2022
@crudiedo
Copy link
Contributor Author

crudiedo commented Oct 10, 2022

Hey @chshersh,

I propose to drop that PR in favor of #135 - even though showing the possible tag is cool, implementing it makes the codebase more complicated and unreadable.

The #135 PR handles both "repo not found" and "tag not found" cases, so it'll be up to the user to check the available tags and choose the one they need.

@chshersh
Copy link
Owner

@crudiedo I agree with your assessment 🙂
Feel free to close this PR then 👍🏻

@crudiedo
Copy link
Contributor Author

Closed due to #135

@crudiedo crudiedo closed this Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted https://hacktoberfest.com/participation/ output Fancy (and not so) output of the tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve error messages when the tag is not found
2 participants